Skip to content

Conversation

@mikeprosserni
Copy link
Collaborator

@mikeprosserni mikeprosserni commented Aug 1, 2025

What does this Pull Request accomplish?

This PR in ni-apis made a few tweaks to these proto files. I'm reflecting those changes here because there were a few changes to functionality that I'd like to get sorted out now. Soon, these protos will be sourced from ni-apis, but I'd like that change to be as simple as a possible when it happens.

This PR makes these changes:

  • panel_uri is now panel_url. No change in functionality, just a rename
  • panel_script_path is now panel_script_url. That also means it should be "file://c:/panel.py" instead of just "c:/panel.py"
  • python_path is now python_interpreter_url. That also means it should be "file://c:/python.exe" instead of just "c:/python.exe"

The corresponding changes to the server are here.

Why should this Pull Request be merged?

We want the proto files to be up to date.

What testing has been done?

image

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

Test Results

   10 files  ± 0     10 suites  ±0   21s ⏱️ -10s
  225 tests + 1    225 ✅ + 1  0 💤 ±0  0 ❌ ±0 
2 200 runs  +10  2 200 ✅ +10  0 💤 ±0  0 ❌ ±0 

Results for commit 2abee34. ± Comparison against base commit 5ade5a6.

This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.
tests.unit.test_streamlit_panel ‑ test___panel___python_path_is_in_venv
tests.unit.test_streamlit_panel ‑ test___panel___python_interpreter_url_is_in_venv
tests.unit.test_streamlit_panel ‑ test___panel___python_script_url_starts_with_file

♻️ This comment has been updated with latest results.

@mikeprosserni mikeprosserni marked this pull request as ready for review August 1, 2025 21:48
@mikeprosserni mikeprosserni requested a review from csjall as a code owner August 1, 2025 21:48
@mikeprosserni mikeprosserni merged commit 39f710e into main Aug 5, 2025
14 checks passed
@mikeprosserni mikeprosserni deleted the users/mprosser/proto-updates branch August 5, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants